Introduce a default_server_name for aesthetics and rework .well-known #2327
Conversation
Fixes element-hq/element-web#7724 The `default_server_name` from the config gets displayed in the "Login with my [server] matrix ID" dropdown when the default server is being used. At this point, we also discourage the use of the `default_hs_url` and `default_is_url` options because we do an implicit .well-known lookup to configure the client based on the `default_server_name`. If the URLs are still present in the config, we'll honour them and won't do a .well-known lookup when the URLs are mixed with the new server_name option. Users will be warned if the `default_server_name` does not match the `default_hs_url` if both are supplied. Users are additionally prevented from logging in, registering, and resetting their password if the implicit .well-known check fails - this is to prevent people from doing actions against the wrong homeserver. This relies on matrix-org/matrix-js-sdk#799 as we now do auto discovery in two places. Instead of bringing the .well-known out to its own utility class in the react-sdk, we might as well drag it out to the js-sdk.
|
At the risk of being late to the party, this seems like it's burdening the user with a the problem of Additional sadness expressed at porting all the same .well-known logic into js-sdk while MSC1700 is still outstanding. |
|
The feature we wanted was to re-label the The above also means that the With matrix-org/synapse#4262 though, maybe we can get away with complaining about it being a configuration error rather than trying to support re-labelling the dropdown.
Eternal sadness is a good theme for this whole epic. Particularly now that I have to fix the js-sdk to not complain about es6 dependencies :( |
|
After discussion we ended up going the route of independent options. See element-hq/element-web#7724 (comment) for details. I'll update this PR accordingly. |
They are now independent of each other. If both are specified in the config, the user will see an error and be prevented from logging in. The expected behaviour is that when a default server name is given, we do a .well-known lookup to find the default homeserver (and block the UI while we do this to prevent it from using matrix.org while we go out and find more information). If the config specifies just a default homeserver URL however, we don't do anything special.
To give the user a little feedback about something happening. This definitely needs to be improved in the future though.
| @@ -1732,6 +1760,28 @@ export default React.createClass({ | |||
| this.setState(newState); | |||
| }, | |||
|
|
|||
| _tryDiscoverDefaultHomeserver: async function(serverName) { | |||
| const discovery = await AutoDiscovery.findClientConfig(serverName); | |||
There was a problem hiding this comment.
Maybe try/catch here, and convert it to a defaultServerDiscoveryError. Otherwise we'd block the UI forever ...
| this.setState({discoveryError: _t("Invalid homeserver discovery response")}); | ||
| return; | ||
| } | ||
| const discovery = await AutoDiscovery.findClientConfig(serverName); |
There was a problem hiding this comment.
Same here, maybe try/catch here, and convert it to a discoveryError. Otherwise we'd block the UI forever ...
| } | ||
| const discovery = await AutoDiscovery.findClientConfig(serverName); | ||
| const state = discovery["m.homeserver"].state; | ||
| if (state !== AutoDiscovery.SUCCESS && state !== AutoDiscovery.PROMPT) { |
There was a problem hiding this comment.
Hmm, a bit of a nit, and probably a bit late to bring this up, but I can't help but feeling this is a lot of code and branching for what we're doing here. Maybe I'm missing something, but AutoDiscovery tries to resolve a server name to a hs/is url, or can fail with a message (which can be fatal or not if we want to to make the FAIL_PROMPT distinction, which seems unused atm). If AutoDiscovery.findClientConfig returns a promise that resolved to an object with hsUrl & isUrl fields or rejects with an error message (and fatal field if needed), much of this code could be simplified to just 1 setState call in the try and one in the catch.
Not too important though, and could have brought this up on the js-sdk side, so feel free to ignore this :)
There was a problem hiding this comment.
The branching looks a bit bad mostly because I ended up using long variable names, which means a lot of line breaks for clarity and to avoid making the linter angry. This is only really handling four cases: failure, success, prompt, and something is really broken (invalid status). We end up treating "prompt" as "please use the custom server options" whereas failure of any kind we fall back on preferring to communicate the problem to the user.
For FAIL_PROMPT, we could handle it as a warning (as it generally only happens when the config is wrong) and let the user go through with the custom server options. However, we don't really have a design for communicating a non-fatal error to the user and encourage use of another component. In the interest of trying to get the user to fix their .well-known config, we take the spec's suggestion to fail to heart and communicate the problem to the user.
In an ideal world, the administrator setting up .well-known would test their changes to make sure they work before their users play with it. Theory is that there are going to be few bad configs out there, and most users will easily breeze through the process anyways.
As for the verboseness: this is an artifact of trying to get all the validation rules communicated and handled in the js-sdk, as desired by the community.
Requires matrix-org/matrix-js-sdk#799
Fixes element-hq/element-web#7724
Note: This PR has changed over time so the description here might be inaccurate. Please read the whole comment chain before assuming the PR description matches the behaviour.
The
default_server_namefrom the config gets displayed in the "Login with my [server] matrix ID" dropdown when the default server is being used. At this point, we also discourage the use of thedefault_hs_urlanddefault_is_urloptions because we do an implicit .well-known lookup to configure the client based on thedefault_server_name. If the URLs are still present in the config, we'll honour them and won't do a .well-known lookup when the URLs are mixed with the new server_name option. Users will be warned if thedefault_server_namedoes not match thedefault_hs_urlif both are supplied. Users are additionally prevented from logging in, registering, and resetting their password if the implicit .well-known check fails - this is to prevent people from doing actions against the wrong homeserver.This relies on matrix-org/matrix-js-sdk#799 as we now do auto discovery in two places. Instead of bringing the .well-known out to its own utility class in the react-sdk, we might as well drag it out to the js-sdk.